Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Namespaced mc #134

Closed
wants to merge 11 commits into from
Closed

Namespaced mc #134

wants to merge 11 commits into from

Conversation

ellemouton
Copy link
Owner

Change Description

Description of change / link to associated issue.

Steps to Test

Steps for reviewers to follow to test the change.

Pull Request Checklist

Testing

  • Your PR passes all CI checks.
  • Tests covering the positive and negative (error paths) are included.
  • Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

📝 Please see our Contribution Guidelines for further guidance.

To prevent the need to copy the entire onion_error.go file for a new
Mission Control migration, this commit just updates the existing
lnwire21/onion_error.go file with the new CodeInvalidBlinding code. The
lnwire21 should not really ever be updated but adding a new code should
be fine as it does not affect old migrations since this is a new code.
In preparation for the commit which will add the main logic for
migration 32 (which will migrate the MC store to use a more minimal
encoding), this commit just adds some of the code that the migration
will need to the package.
Add a new mcRoute type that houses the data about a route that MC
actually uses. Then add a migration (channeldb/migration32) that
migrates the existing store from its current serialisation to the new,
more minimal serialisation.
Copy link

Pull reviewers stats

Stats of the last 30 days for lnd:

User Total reviews Time to review Total comments

@ellemouton ellemouton force-pushed the namespacedMC branch 2 times, most recently from e0de473 to 1842f7d Compare August 13, 2024 14:14
@coveralls
Copy link

coveralls commented Aug 13, 2024

Pull Request Test Coverage Report for Build 10374277490

Details

  • 253 of 980 (25.82%) changed or added relevant lines in 16 files are covered.
  • 22956 unchanged lines in 398 files lost coverage.
  • Overall coverage decreased (-8.4%) to 50.148%

Changes Missing Coverage Covered Lines Changed/Added Lines %
routing/result_interpretation.go 50 55 90.91%
server.go 9 15 60.0%
routing/missioncontrol_store.go 52 70 74.29%
channeldb/migration/lnwire21/onion_error.go 0 21 0.0%
channeldb/migration32/migration.go 0 29 0.0%
channeldb/migration32/hop.go 0 30 0.0%
channeldb/migration33/migration.go 0 34 0.0%
routing/missioncontrol.go 132 166 79.52%
channeldb/migration32/codec.go 0 79 0.0%
channeldb/migration32/mission_control_store.go 0 193 0.0%
Files with Coverage Reduction New Missed Lines %
graph/errors.go 1 94.74%
lnwire/typed_lease_expiry.go 2 78.95%
lnwire/typed_fee.go 2 66.67%
htlcswitch/linkfailure.go 2 51.72%
channeldb/forwarding_policy.go 2 85.14%
lnwallet/musig_session.go 2 79.29%
input/taproot.go 2 93.26%
server.go 2 63.76%
record/hop.go 2 93.33%
lnwire/short_channel_id.go 2 89.74%
Totals Coverage Status
Change from base Build 10369957973: -8.4%
Covered Lines: 96333
Relevant Lines: 192099

💛 - Coveralls

So that `missionControlStore` can be unaware of the backing DB structure
it is writing to. In an upcoming commit when we change mission control
to write to namespaced buckets instead, we then only need to update the
`namespacedDB` implementation.
In this commit, the mission control store is migrated such that all
existing pairs which are currently stored directly in the top level
results bucket are now instead moved to a "default" namespace bucket.

Note that this migration is not yet invoked in this commit. The
migration will be invoked in the same commit that starts writing and
reading the new format.
and invoke the associated mission control migration.
Only the MissionControl instance should use this variable and it should
not be accessible to users of MissionControl.
This commit renames the previous MissionControl to MissionControlManager
since it will soon back multiple namespaced MissionControl instances.
For now, it just houses a single MissionControl in the default
namespace.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants